Intel RDT: updates according to proposed spec changes. #3832
Conversation
ipuustin
force-pushed
the
intel-rdt-updates
branch
from
c486145
to
d34c05a
Compare
|
Rebased. |
There are two proposed clarifications to the OCI spec. The first item specifies that the L3CacheSchema line filtering needs to happen only when both MemBw and L3Cache schemas are specified. The second item specifies that the subdirectory needs to be deleted. Runc already does that, but the clarification adds for directory removal only if the directory was created by us. Signed-off-by: Ismo Puustinen <[email protected]>
ipuustin
force-pushed
the
intel-rdt-updates
branch
from
d34c05a
to
1641f82
Compare
|
Rebased. |
|
The spec PR is not merged yet: |
|
So, this one and #3833 shares some common changes. Which one do you want to go first? |
| if m.config.IntelRdt != nil && m.config.IntelRdt.ClosID == "" { | ||
| // There are probably other containers/tasks sharing the same group. Also | ||
| // only remove the directory if it was created by us. | ||
| if m.config.IntelRdt != nil && m.config.IntelRdt.ClosID == "" && m.directoryCreated { | ||
| m.mu.Lock() | ||
| defer m.mu.Unlock() | ||
| if err := os.RemoveAll(m.GetPath()); err != nil { |
There was a problem hiding this comment.
not really about this PR, but this can benefit from cgroups.RemovePath, as here we only want to remove directories, not files.
| // If both l3CacheSchema and memBwSchema are set and | ||
| // l3CacheSchema contains a line beginning with "MB:", the | ||
| // value written to schemata file MUST be the non-"MB:" | ||
| // line(s) from l3CacheSchema and the line from memBWSchema. | ||
|
|
||
| validLines := make([]string, 0) | ||
|
|
||
| // Split the l3CacheSchema to lines. | ||
| lines := strings.Split(l3CacheSchema, "\n") | ||
|
|
||
| // Remove the "MB:" lines. | ||
| for _, line := range lines { | ||
| if strings.HasPrefix(line, "MB:") { | ||
| continue | ||
| } | ||
| validLines = append(validLines, line) | ||
| } | ||
|
|
||
| return strings.Join(validLines, "\n") + "\n" + memBwSchema |
There was a problem hiding this comment.
This is not very efficient in terms of resources. We split the strings and then we combine them, creating two slices in the process. I'm not sure if there's a better solution (except than a slight optimization of reusing the same slice -- see https://github.com/golang/go/wiki/SliceTricks#filtering-without-allocating).
| // If both l3CacheSchema and memBwSchema are set and | ||
| // l3CacheSchema contains a line beginning with "MB:", the | ||
| // value written to schemata file MUST be the non-"MB:" | ||
| // line(s) from l3CacheSchema and the line from memBWSchema. |
There was a problem hiding this comment.
So, can you elaborate on that requirement? In particular, why we allow a user to include MB: lines into l3CacheSchema? Would it be better to offload this requirement to a user of runc (whoever creates the OCI spec)? I mean, runc is a low level tool and should, theoretically, be as simple as possible.
My other question is -- what happens if we write both values as they are (i.e. without filtering)? Is this a security risk? Or is it just a convenience feature that allows memBwSchema to overwrite whatever is in l3CacheSchema?
There was a problem hiding this comment.
One other thought. Let's say that we have
memBwSchema = "MB:0=70;1=20"
l3CacheSchema = "MB:0=80;1=10\nL3:0=f0;1=f"
Does it make a difference for the kernel, if we write
MB:0=80;1=10
L3:0=f0;1=f
MB:0=70;1=20
or just
L3:0=f0;1=f
MB:0=70;1=20
?
(I mean, except the additional line parsing done in the kernel)
There was a problem hiding this comment.
To answer your other question: I tried this out now and having two MB values for the same domain in the same write won't work:
[root@wolfpass ipuustin]# uname -r
6.0.10-100.fc35.x86_64
[root@wolfpass ipuustin]# cat schemata-1.txt
MB:0=80;1=90
L3:0=ff;1=ff
[root@wolfpass ipuustin]# cat schemata-2.txt
MB:0=80;1=90
L3:0=ff;1=ff
MB:0=60;1=70
[root@wolfpass ipuustin]# cat schemata-1.txt > /sys/fs/resctrl/foo/schemata
[root@wolfpass ipuustin]# cat /sys/fs/resctrl/foo/schemata
MB:0= 80;1= 90
L3:0=0ff;1=0ff
[root@wolfpass ipuustin]# cat schemata-2.txt > /sys/fs/resctrl/foo/schemata
cat: write error: Invalid argument
[root@wolfpass ipuustin]# cat /sys/fs/resctrl/info/last_cmd_status
Duplicate domain 0
So we know that there is nobody trying to set overlapping MB: values in both memBwSchema and l3CacheSchema because that would now trigger an error in runc. There might be someone setting non-overlapping MB: values and not getting those filtered out however.
|
Moving to 1.3.0, since the spec change isn't out and we want a 1.2.0 release soon. |
There are two proposed clarifications to the OCI spec here: opencontainers/runtime-spec#1196
The first item specifies that the L3CacheSchema line filtering needs to happen only when both MemBw and L3Cache schemas are specified.
The second item specifies that the subdirectory needs to be deleted. Runc already does that, but the clarification adds for directory removal only if the directory was created by us.